-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Topic refactor #3483
Topic refactor #3483
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Requesting changes to have broadcast da also accept a topic. Also given the calls to the network are already split, do we even need to pass a topic in as an argument to the connected network? Either way broadcast_message
and broadcast_da_message
should have the same signature, or be combined again
async fn broadcast_message( | ||
&self, | ||
message: Vec<u8>, | ||
recipients: BTreeSet<K>, | ||
topic: Topic, | ||
broadcast_delay: BroadcastDelay, | ||
) -> Result<(), NetworkError>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did broadcast DA not also change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DA broadcast is a direct send to all recipients instead of an actual broadcast
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should remove da_broadcast_message
and conditionally on the topic have the DA specific logic when needed (as in Libp2p)
Closes #3485
This PR:
Refactors all networks to broadcast messages through
Topics
instead of a static list of all recipients. This will be needed to support users dynamically leaving and joining the networkThis PR does not:
Make any breaking protocol changes
Key places to review:
libp2p_network.rs
,memory_network.rs